Skip to content

tests: unit: ble_adv: improvements to ble_adv testcase configs#656

Merged
eivindj-nordic merged 2 commits intonrfconnect:mainfrom
anhmolt:fix-ble-adv-unit-test-config
Feb 26, 2026
Merged

tests: unit: ble_adv: improvements to ble_adv testcase configs#656
eivindj-nordic merged 2 commits intonrfconnect:mainfrom
anhmolt:fix-ble-adv-unit-test-config

Conversation

@anhmolt
Copy link
Contributor

@anhmolt anhmolt commented Feb 23, 2026

First commit fixes missing directed advertising configuration for two of the test cases.
Second commit reworks the testcases to list what modes are enabled instead of listing what modes are disabled.

@anhmolt anhmolt requested a review from a team as a code owner February 23, 2026 11:49
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Feb 23, 2026
@NordicBuilder NordicBuilder requested a review from a team February 23, 2026 11:49
@anhmolt anhmolt removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Feb 23, 2026
@anhmolt
Copy link
Contributor Author

anhmolt commented Feb 23, 2026

Changes to a unit test. No changelog entry needed.

@github-actions
Copy link

You can find the documentation preview for this PR here.

@lemrey
Copy link
Contributor

lemrey commented Feb 23, 2026

Second commit reworks the testcases to list what modes are enabled instead of listing what modes are disabled.

Does it? It seems that we disable some modes instead of enabling the mode we want to test.

  lib.ble_adv.mode_fast:
    extra_configs:
      - CONFIG_BLE_ADV_SLOW_ADVERTISING=n
  lib.ble_adv.mode_slow:
    extra_configs:
      - CONFIG_BLE_ADV_FAST_ADVERTISING=n

@anhmolt
Copy link
Contributor Author

anhmolt commented Feb 23, 2026

Second commit reworks the testcases to list what modes are enabled instead of listing what modes are disabled.

Does it? It seems that we disable some modes instead of enabling the mode we want to test.

  lib.ble_adv.mode_fast:
    extra_configs:
      - CONFIG_BLE_ADV_SLOW_ADVERTISING=n
  lib.ble_adv.mode_slow:
    extra_configs:
      - CONFIG_BLE_ADV_FAST_ADVERTISING=n

I was referring to the "list" of modes in the testcase names. Not what configs are disabled or enabled.

@@ -4,24 +4,29 @@ common:
tags: unittest
Copy link
Contributor

@eivindj-nordic eivindj-nordic Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, will discuss offline, would it be an idea to turn off all modes in prj.conf, then enable everything here based on the test case? Could give a better overview of what is enabled for the different tests.

Copy link
Contributor Author

@anhmolt anhmolt Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The second commit now disables every advertising mode in prj.conf so that the desired modes can be (re)enabled in testcase.yaml. I think it should be more readable and maintainable now.

@anhmolt anhmolt force-pushed the fix-ble-adv-unit-test-config branch from 6dc22d7 to 82f6b65 Compare February 24, 2026 13:37
The no_directed_hd_or_fast and no_directed_hd_or_slow test
configurations was missing CONFIG_BLE_ADV_DIRECTED_ADVERTISING=y.

In addition, align the order of configs.

Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
To increase readability and maintainability, rework testcase logic
to list what modes are enabled instead of listing what modes are
disabled. To achieve this, disable all advertising modes in the
prj.conf file.

Signed-off-by: Andreas Moltumyr <andreas.moltumyr@nordicsemi.no>
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Feb 24, 2026
@anhmolt anhmolt force-pushed the fix-ble-adv-unit-test-config branch from 82f6b65 to b925db4 Compare February 24, 2026 13:38
@eivindj-nordic eivindj-nordic removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Feb 24, 2026
@eivindj-nordic eivindj-nordic merged commit 0498758 into nrfconnect:main Feb 26, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants